feat(openbao): add imagePullSecrets + use mirrored bank vaults operator#545
feat(openbao): add imagePullSecrets + use mirrored bank vaults operator#545Jcing95 wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
This PR makes the oms install openbao installer configurable for mirrored/private OCI registries by adding image/chart override options, wiring an optional registry pull secret into both the OpenBao ServiceAccount and the operator chart, and authenticating Helm against the operator chart registry before pulling.
Changes:
- Added installer support for
OMS_REGISTRY_USER/OMS_REGISTRY_PASSWORDto create/update adockerconfigjsonpull secret and wire it into the OpenBao ServiceAccount and operator Helm values. - Added CLI flags to override OpenBao/bank-vaults/operator images and the operator chart repo, with default backfill in config validation.
- Updated the default vault-operator chart repo/image to the Codesphere mirror and bumped the chart/operator version to
1.24.0, plus updated docs/tests.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| internal/installer/openbao.go | Adds registry credential handling, pull-secret creation, Helm OCI login for chart pulls, configurable image/chart refs, and operator chart value overrides. |
| internal/installer/openbao_test.go | Adds/updates tests for mirrored overrides, pull-secret behavior, host de-duplication, and config default backfill. |
| internal/installer/manifests/openbao/vault-cr.yaml | Wires imagePullSecrets into the openbao ServiceAccount template conditionally. |
| docs/oms_install_openbao.md | Documents new flags and registry credential env vars, with mirrored-registry example. |
| cli/cmd/install_openbao.go | Adds new CLI flags and passes env-based registry credentials + override fields into installer config. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Update existing secret — preserve metadata, refresh type and data. | ||
| existing.Type = corev1.SecretTypeDockerConfigJson | ||
| existing.Data = map[string][]byte{corev1.DockerConfigJsonKey: dockerConfig} | ||
| if _, err := secretsClient.Update(o.ctx, existing, metav1.UpdateOptions{}); err != nil { | ||
| return fmt.Errorf("updating image pull secret: %w", err) | ||
| } |
OliverTrautvetter
left a comment
OliverTrautvetter
left a comment
There was a problem hiding this comment.
Looks mostly good 👍 some smaller comments
| if o.Config.RegistryUser == "" && o.Config.RegistryPassword == "" { | ||
| return nil | ||
| } | ||
| if !o.imagePullSecretConfigured() { | ||
| return fmt.Errorf("incomplete registry credentials: set both OMS_REGISTRY_USER and OMS_REGISTRY_PASSWORD, or neither") | ||
| } |
There was a problem hiding this comment.
Can be simplified.
imagePullSecretConfigured already checks o.Config.RegistryUser != "" && o.Config.RegistryPassword != ""
| return fmt.Errorf("failed to ensure namespace: %w", err) | ||
| } | ||
|
|
||
| // Create the GHCR pull secret before the operator schedules any pods, so |
There was a problem hiding this comment.
is it working just for GHCR? Does it not work for any registry host derived from the image refs?
| openbao.cmd.Flags().StringVar(&openbao.Opts.OpenBaoImage, "openbao-image", installer.DefaultOpenBaoImage, "OpenBao server image (override for a mirrored OCI registry)") | ||
| openbao.cmd.Flags().StringVar(&openbao.Opts.BankVaultsImage, "bank-vaults-image", installer.DefaultBankVaultsImage, "Bank-Vaults configurer image (override for a mirrored OCI registry)") | ||
| openbao.cmd.Flags().StringVar(&openbao.Opts.OperatorImage, "operator-image", installer.DefaultOperatorImage, "Bank-Vaults operator pod image (override for a mirrored OCI registry)") | ||
| openbao.cmd.Flags().StringVar(&openbao.Opts.OperatorChartRepo, "operator-chart-repo", installer.DefaultBankVaultsChartRepo, "OCI repo hosting the vault-operator Helm chart (override for a mirrored OCI registry)") |
There was a problem hiding this comment.
Should we check if it starts with oci:// ? In validateConfig()
something like
if !strings.HasPrefix(o.Config.OperatorChartRepo, "oci://") {
return fmt.Errorf("--operator-chart-repo must use oci:// scheme, got %q", o.Config.OperatorChartRepo)
}
| if tagged, ok := ref.(reference.Tagged); ok { | ||
| image["tag"] = tagged.Tag() | ||
| } |
There was a problem hiding this comment.
Maybe check if a tag is set. So return an error if the image reference has neither a tag nor a digest
| // references (e.g. "ghcr.io/codesphere-cloud/.../openbao:2.5.4" -> "ghcr.io"). | ||
| // Empty refs are skipped. The result is order-stable so the rendered secret is | ||
| // deterministic across runs. | ||
| func registryHostsFor(images ...string) ([]string, error) { |
There was a problem hiding this comment.
Maybe add some test for digest-only references (like registry.example.com/image@sha256:abc...)
What
Makes the
oms install openbaoimage and chart sources configurable for mirrored OCI registries, and adds an image pull secret so the OpenBao, bank-vaults, and operator images can be pulled from a private registry.Why
The OpenBao server, bank-vaults configurer, and operator images already ship from the Codesphere GHCR mirror, but there was no way to authenticate pulls against a private registry, and the operator Helm chart was still pulled from public
ghcr.io/bank-vaults. This lets a cluster (or a customer using their own mirror) pull every artifact from a single private registry.Changes
--openbao-image,--bank-vaults-image,--operator-image,--operator-chart-repo. Empty values are backfilled from theDefault*constants invalidateConfig, so programmatic callers and the default install are unaffected.OMS_REGISTRY_USERandOMS_REGISTRY_PASSWORDare both set, the installer creates/updates adockerconfigjsonsecret (openbao-registry) with oneauthsentry per distinct registry host derived from the configured image refs. It is:openbaoServiceAccount (via the Vault CR template), andimage.imagePullSecretsvalues.pc_apps.gopattern, so the chart can be pulled from a private OCI repo.1.22.5→1.24.0, default operator imagevault-operator:1.24.0. Default operator chart repo now points at the Codesphere mirror (oci://ghcr.io/codesphere-cloud/docker/ghcr.io/bank-vaults/helm-charts).docs/oms_install_openbao.mdwith the new flags, the env vars, and a mirrored-registry example.Notes for reviewers
OMS_REGISTRY_USER/OMS_REGISTRY_PASSWORDunless the mirror packages are public-read. Node-level registry creds do not help the client-side chart pull.image.repository/image.tag/image.imagePullSecretsper the vault-operator chart schema; a chart version bump could move these keys (wrong keys are silently ignored and fall back to chart defaults). Verify withhelm show values oci://ghcr.io/bank-vaults/helm-charts/vault-operator --version 1.24.0.Testing
go build ./...,go test ./internal/installer/...pass;gofmtclean.validateConfigdefault backfill, and ServiceAccountimagePullSecretstemplate rendering (set and unset).